askrene: opt-in circular (self-rebalance) routes in getroutes#9286
askrene: opt-in circular (self-rebalance) routes in getroutes#9286ksedgwic wants to merge 7 commits into
Conversation
source == destination is rejected as invalid input: a node cannot pay
itself a normal route. This adds an opt-in, allow_circular: when set,
source == destination instead requests a self-rebalance cycle -- a route
that leaves source, traverses the network, and returns to source -- for
moving liquidity between one's own channels.
Without allow_circular nothing changes: source == destination still fails
with "source and destination must be different". Only callers that
explicitly opt in are affected, so this is purely additive for every
existing caller.
The cycle is produced by splitting the source node into two distinct nodes
at the gossmap-localmods layer, before the solver child forks:
- source itself keeps only its outgoing edges (its real channels, modulo
any masks the caller applied via layers).
- A synthetic us_in node receives only incoming edges: one mirror per
still-enabled (peer -> source) direction, with the real channel's
capacity, fees, cltv delta and htlc bounds copied over.
The MCF then solves a regular source -> us_in flow. No direct edge
connects them, so the algorithm must traverse the network, and every route
ends at us_in with a non-empty hop sequence. The whole change lives in the
request-setup layer in askrene.c; the solver child (MCF / BFS / Dijkstra /
get_flow_paths / find_path_or_cycle / substract_flow / substract_cycle /
routing-cost machinery and route serialisation) runs unmodified, because
us_in is a real entry in the mod-augmented gossmap, indexed in the usual
range and picked up by every loop over gossmap_max_node_idx.
The trailing (peer -> us_in) hop is KEPT in each returned route: its
amount_out_msat is the delivered amount, and its short_channel_id_dir and
node_id_out are sentinel placeholders for the synthetic us_in node. The
caller replaces them with the real closing channel and its own node id when
building the payment, closing the cycle.
Fake mirror scids count up from 0x0x0, skipping any scid already present in
the gossmap or the request's localmods: block 0 cannot hold a real mainnet
channel, but synthetic gossmaps and layer-created channels can occupy any
scid, and a clash would silently update the occupant instead of creating
the mirror. Allocation failure (or any other mirror-construction failure)
fails the getroutes command cleanly rather than aborting, since cln-askrene
is an important plugin and an abort would take lightningd down with it.
Changelog-Added: JSON-RPC: `getroutes` `allow_circular` returns a self-rebalance cycle when `source` equals `destination`.
Add integration coverage for the allow_circular self-rebalance path: - test_getroutes_circular: a focused test over a synthetic gossip store with one deterministic cycle (us -> 1 -> 2 -> us). Asserts that source == destination without allow_circular is rejected, and that with allow_circular the returned route keeps its trailing synthetic closing hop (node_id_out == the fake us_in sentinel, scid == the first mirror 0x0x0, amount_out_msat == delivered). Also covers mirror-scid collision handling: a layer-created channel squatting on 0x0x0 plus the store's own real block-0 channel (gossmap-compress assigns block = index + node1) force the allocator to skip to 0x2x0. - test_real_data_circular: the canonical clboss-xmovefunds shape over a real mainnet gossmap snapshot. Mask the source node down to a single drain (source-out) channel and a single distinct fill (dest-in) channel, then self-rebalance. The mask makes the route deterministic at the endpoints: every part must leave via the drain channel and return via the mirrored fill channel into us_in. Real hops in the synthetic test are pinned by node id rather than short_channel_id_dir, since the scids the test gossip store assigns are a harness-internal detail. Changelog-None
a287403 to
65d4a82
Compare
|
rebased, hoping for a good CI run ... |
|
This is an excellent PR!
|
| ], | ||
| "default": "100" | ||
| }, | ||
| "allow_circular": { |
There was a problem hiding this comment.
We can be increasing the number of flags forever. That's why, I guess, Rusty preferred
to introduce these as auto layers. So I would prefer to adhere to that pattern.
Instead of a boolean option this would be a auto.allow_circular layer.
There was a problem hiding this comment.
Agreed, this is much better.
Adopted in "askrene: replace allow_circular parameter with auto.allow_circular layer". The layer form also fits how circular requests are actually made: the caller is already composing a layers list to pin which channels the cycle drains and fills, so the opt-in travels with the rest of the request shape. A side benefit: the parameter's footprint in the generated bindings (msggen, grpc, pyln) disappears entirely, and since the field numbering never shipped in a release it could be dropped cleanly.
| /* Self-rebalance handling: when source == destination, splice | ||
| * a fake "us_in" destination node into localmods and mirror | ||
| * the still-enabled (peer -> source) directions onto it. | ||
| * Algorithms see a regular s -> t flow problem and run | ||
| * unchanged. Must precede gossmap_apply_localmods so the | ||
| * augmented mods are picked up by the apply below. Only | ||
| * reached for source == destination when the caller passed | ||
| * allow_circular (json_getroutes rejects it otherwise). */ | ||
| err = inject_circular_fake(info, askrene->gossmap, localmods); | ||
| if (err) { | ||
| /* localmods are not applied at this point, so fail | ||
| * directly: the fail: path would try to remove them. */ | ||
| return command_fail(cmd, PAY_ROUTE_NOT_FOUND, "%s", err); | ||
| } | ||
| if (info->circular) | ||
| cmd_log(tmpctx, cmd, LOG_DBG, | ||
| "Circular routing: spliced fake us_in destination " | ||
| "node, mirrored peer -> source channels into it"); | ||
|
|
There was a problem hiding this comment.
In this section, isn't it better to have the following pattern: ?
if(node_id_eq(source, dest)){
// log source==dest -> needs circular ...
err = inject_circular_fake(...);
if(err){
// log needs circular but failed for reason: {err}
return command_fail(...);
}
// log circular routing prepared ...
}There was a problem hiding this comment.
Adopted in "askrene: make the circular case explicit at the inject_circular_fake call site". This came out cleaner than expected: the info->circular field was only read back for a single debug log, so hoisting the source == destination condition to the call site deletes the field, its initialization, and the post-hoc log entirely. inject_circular_fake() keeps an internal guard as a defensive measure. It also makes the layer structurally inert for normal requests -- nothing circular runs unless source == destination -- which the follow-up commit documents in the schema and pins with an equivalence test.
|
I remember @daywalker90 raised a question about circular rebalancing using I would like to also point out that the primitive API already present in My suggestion is to do two things:
|
The layer form also composes naturally: circular requests already require companion layers to pin which channels the cycle drains and fills, so callers are working with the layers list anyway. A side benefit: the parameter's footprint in the generated bindings (.msggen.json, cln-grpc proto/convert, cln-rpc model, pyln-client and pyln-grpc-proto) disappears entirely -- a layer name needs no binding support. The allow_circular field numbering is dropped from .msggen.json since it never shipped in a release. This supersedes the Changelog-Added line in the commit that introduced allow_circular; fold when squashing. Changelog-Added: JSON-RPC: `getroutes` new auto.allow_circular layer returns a self-rebalance cycle when `source` equals `destination`.
…call site Per review: instead of inject_circular_fake() silently no-oping for normal requests, test source == destination at the do_getroutes call site, log each stage (requested / failed / prepared), and only call the function for circular requests. This turns out to be a net simplification: the info->circular flag was only ever read for the post-hoc success log, so the explicit call-site condition lets us delete the struct field, its comment block, its initialization, and the function's early-return guard. The function's contract (only called for source == destination) is now visible in the caller. Changelog-None
Previously each circular route ended in a sentinel final hop: a fake mirror scid and a placeholder node id the caller had to recognise and replace with a real return channel and their own node id. That contract had a hole when more than one (peer -> source) channel from the same final peer is left enabled -- e.g. parallel channels, or letting the solver split the fill across several: the mirror-to-real mapping exists only inside askrene, and parallel channels' properties typically tie, so the caller cannot reconstruct which real channel each route's solve targeted. Non-strict forwarding usually still delivers the funds, but the rebalance loses exactly what it is for: which channel gets filled, the per-channel split the solver planned, and scid-keyed bookkeeping (reservations, failure feedback) on the final hop. Record the mapping at inject time (struct circular_unsplit: one entry per mirror, fake scid -> real scidd, plus the real source id) and apply it in the solver child when serializing routes: any hop over a mirror's fake scid -- only final hops can match, by construction -- is rewritten to the real (peer -> source) return channel with node_id_out set to source. The table is built in the parent before the fork, so the child inherits it. Returned circular routes are now directly usable for building the payment; no placeholder substitution is required, and the sentinel never appears in output. The new multi-inbound test covers the previously unanswerable case: an amount that must split across two inbound channels from the same final peer, with each route's final hop carrying the correct distinct real scid. Changelog-None
… routes The auto.allow_circular layer grants permission (source may equal destination); it does not change behavior. On a normal request, where source and destination differ, the layer must be a no-op. This is structural: the circular setup runs only behind the source == destination check at the do_getroutes call site. Say so in the schema documentation, and pin it with an equivalence test: getroutes returns the identical result with and without the layer on a non-circular request. Changelog-None
|
Pushed four commits:
Kept as separate commits so each response reviews as its own diff; happy to squash or reorder at merge time ... |
|
The idea of encapsulating the next layer of rebalancing mechanism as a plugin or modification to existing code certainly resonates. CLBOSS has a special layer to hold the fee-optimized, more patient rebalancing attempts. We could package:
I'm guessing we still want to support different high-level strategies for rebalancing, but maybe they all share the basic mechanics? Would this be a follow-on PR? it's sort of a layer on top of this one ... |
The final-hop translation commit added common/node_id.h to child/child.h for struct circular_unsplit; child.c already included it directly, and check-includes rejects a .c file re-including what its own header provides. Drop the direct include. Fixes the check-source failure in CI. Changelog-None
This adds an opt-in allow_circular parameter to askrene-getroutes. When set, source == destination requests a self-rebalance cycle -- a route that leaves the source, traverses the network, and returns -- for moving liquidity between one's own channels. Because the cycles come from the MCF solver, a single call can move liquidity out of several channels and into several others in one pass, rather than rebalancing one channel pair per attempt. Without the flag nothing changes: source == destination is still rejected with the same error as today, so existing callers are unaffected.
The change stays out of the solver. It uses the standard node-splitting construction from network flow: at the gossmap-localmods layer, the source keeps only its outgoing edges and a synthetic sink receives mirrored copies of its incoming edges, so the MCF sees an ordinary source-to-sink problem and the solver child runs unmodified. The commit messages have the details, including how the returned route represents the closing hop.
Motivation: rebalancing tools currently have to do their own routing for circular payments; this lets them use the same solver as everything else. We have been running this patch against live rebalance traffic on mainnet (and signet).
Tests cover the flag-off (unchanged) behavior, a deterministic synthetic cycle, mirror-scid collision handling, and a self-rebalance over a real gossmap snapshot.
Changelog-Added: JSON-RPC:
askrene-getrouteshas a newallow_circularparameter to request self-rebalance (source == destination) routes.